Append client version info to user graffiti by default#9313
Conversation
chong-he
added
val-client
|
|
||
| #[clap( | ||
| long, | ||
| requires = "graffiti", |
There was a problem hiding this comment.
Remove this requires, this is for users who does not want to have any graffiti at all, so they can use --graffiti-append false without needing the --graffiti flag
| &randao_reveal, | ||
| graffiti.as_ref(), | ||
| SkipRandaoVerification::No, | ||
| SkipRandaoVerification::Yes, |
There was a problem hiding this comment.
| SkipRandaoVerification::Yes, | |
| SkipRandaoVerification::No, |
Should leave as is
There was a problem hiding this comment.
Looking at this, I thought I wanted to be consistent with other variables of using SkipRandaoVerification::Yes, that's probably why I changed to yes. As this is a test that is not related to RANDAO, I guess it is ok to use Yes here?
(i.e., the previous No is probably not quite right)
| requires = "graffiti", | ||
| help = "When used, client version info will be prepended to user custom graffiti, with a space in between. \ | ||
| help = "When used, client version info will be appended to user custom graffiti, with a space in between. \ | ||
| This should only be used with a Lighthouse beacon node.", |
There was a problem hiding this comment.
Should tweak this to say:
This should only be set to
falsewhen using a Lighthouse beacon node.
And something about how the default behaviour is to append.
There was a problem hiding this comment.
I added a default value to the flag in the cli to make it clearer
| #[test] | ||
| fn graffiti_append_true_flag() { | ||
| CommandLineTest::new() | ||
| .flag("graffiti-append", Some("true")) |
There was a problem hiding this comment.
Also need to test .flag("graffiti-append", Some("true")) which corresponds to just --graffiti-append. Some people might be using that from their old config.
There was a problem hiding this comment.
Added a test to prevent regression
| `This is my graffiti string yo yo` | ||
|
|
||
| To opt out from this, use the flag `--graffiti-append false` on the validator client. This will retain your own | ||
| graffiti when proposing a block, without appending any client version info. |
There was a problem hiding this comment.
Should highlight that this ONLY works with Lighthouse BNs
There was a problem hiding this comment.
Revised the documentation to make this clear
michaelsproul
added
waiting-on-author
chong-he
added
ready-for-review
2 participants
Issue Addressed
Closes issue #9287
Additional Info
By default, append client version info by including the client code and some commit hash of the version, see: https://hackmd.io/@wmoBhF17RAOH2NZ5bNXJVg/BJX2c9gja
The exact implementation on how to append the client version info is different from other clients, e.g., Teku: https://docs.teku.consensys.io/reference/cli#validators-graffiti-client-append-format
I think that should be ok, as long as the appended info contains the EL, CL and the version (commit hash).